test(e2e): report real zero-touch capability to WebKit for command-surface specs#995
Conversation
|
This pull request has been ignored for the connected project Preview Branches by Supabase. |
📝 WalkthroughWalkthroughUI test suites now override ChangesBrowser touch capability normalization
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested labels: Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
bac281d to
96020d7
Compare
…rface specs Playwright's Linux WebKit build advertises phantom touch points on the touch-free CI runner. commandDropdownCanDisplay deliberately blocks wide touch devices (fine-pointer OR zero-touch required), and headless browsers fail the fine-pointer media query — so Chromium and Firefox pass via the zero-touch fallback while WebKit flunks both branches and never renders the desktop command dropdown. All 8 ui-universal-search WebKit failures in matrix run 4012 share that one signature (suggestion content never enters the DOM), with candidates across the other ui specs. A beforeEach init script in the six ui spec files stubs navigator.maxTouchPoints to the runner's true value (0); the product gate and its unit pins are unchanged, and Chromium/Firefox already report 0 so the stub is inert there. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
96020d7 to
70750be
Compare
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/ui-accessibility.spec.ts (1)
181-190: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExtract the zero-touch stub into a shared helper to eliminate duplication across 6 spec files.
The same
addInitScriptblock (and a near-identical 4-line comment) is copy-pasted across all six UI specs. If the stubbing approach ever needs adjustment, it must be changed in 6 places with risk of drift. Extract a single helper and import it where needed.
tests/ui-accessibility.spec.ts#L181-L190: replace the standalonebeforeEachbody with a call to the shared helper.tests/ui-formulation.spec.ts#L53-L59: replace the inlineaddInitScriptcall with the shared helper inside the existingbeforeEach.tests/ui-smoke.spec.ts#L876-L885: replace the standalonebeforeEachbody with a call to the shared helper.tests/ui-stress.spec.ts#L324-L333: replace the standalonebeforeEachbody with a call to the shared helper.tests/ui-tools.spec.ts#L353-L362: replace the standalonebeforeEachbody with a call to the shared helper.tests/ui-universal-search.spec.ts#L118-L129: replace the standalonebeforeEachbody with a call to the shared helper.♻️ Proposed shared helper
// tests/helpers/zero-touch.ts import type { Page } from "`@playwright/test`"; /** * Playwright's Linux WebKit build advertises phantom touch points on the touch-free CI * runner, tripping the fine-pointer/zero-touch gate on the search command surface * (commandDropdownCanDisplay) that Chromium and Firefox pass via the zero-touch fallback. * Report the runner's real capability so WebKit exercises the same desktop surfaces. */ export async function stubZeroTouchPoints(page: Page) { await page.addInitScript(() => { Object.defineProperty(Navigator.prototype, "maxTouchPoints", { configurable: true, get: () => 0, }); }); }Usage in standalone-
beforeEachfiles:+import { stubZeroTouchPoints } from "./helpers/zero-touch"; + -test.beforeEach(async ({ page }) => { - await page.addInitScript(() => { - Object.defineProperty(Navigator.prototype, "maxTouchPoints", { configurable: true, get: () => 0 }); - }); -}); +test.beforeEach(stubZeroTouchPoints);Usage in
tests/ui-formulation.spec.ts(inside existingbeforeEach):+import { stubZeroTouchPoints } from "./helpers/zero-touch"; + // ...existing setup... - await page.addInitScript(() => { - Object.defineProperty(Navigator.prototype, "maxTouchPoints", { configurable: true, get: () => 0 }); - }); + await stubZeroTouchPoints(page);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@tests/ui-accessibility.spec.ts` around lines 181 - 190, Extract the duplicated zero-touch initialization into a shared stubZeroTouchPoints helper, preserving the existing maxTouchPoints behavior and comment. Update tests/ui-accessibility.spec.ts (181-190), tests/ui-formulation.spec.ts (53-59), tests/ui-smoke.spec.ts (876-885), tests/ui-stress.spec.ts (324-333), tests/ui-tools.spec.ts (353-362), and tests/ui-universal-search.spec.ts (118-129) to import and call the helper; integrate it within formulation’s existing beforeEach and replace the standalone beforeEach bodies in the other files.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@tests/ui-accessibility.spec.ts`:
- Around line 181-190: Extract the duplicated zero-touch initialization into a
shared stubZeroTouchPoints helper, preserving the existing maxTouchPoints
behavior and comment. Update tests/ui-accessibility.spec.ts (181-190),
tests/ui-formulation.spec.ts (53-59), tests/ui-smoke.spec.ts (876-885),
tests/ui-stress.spec.ts (324-333), tests/ui-tools.spec.ts (353-362), and
tests/ui-universal-search.spec.ts (118-129) to import and call the helper;
integrate it within formulation’s existing beforeEach and replace the standalone
beforeEach bodies in the other files.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 1084c161-7822-4333-b39b-a6c06e693d40
📒 Files selected for processing (7)
docs/branch-review-ledger.mdtests/ui-accessibility.spec.tstests/ui-formulation.spec.tstests/ui-smoke.spec.tstests/ui-stress.spec.tstests/ui-tools.spec.tstests/ui-universal-search.spec.ts
…ck, WebKit focus/forced-colors/stale-style, Firefox tab order, CIWA term alias Seven fixes from the run-4012 triage, each root-caused: - playwright.config.ts: serviceWorkers 'block' suite-wide (+ ui-pwa.spec.ts 'allow' opt-in). In production builds public/sw.js registers in every test, claims the page (clients.claim) and serves every navigation — probe-proven to bypass route interception for navigations on Chromium and to wedge Playwright-Firefox's reload path under an active route (both ui-smoke reload hangs). Only ui-pwa exercises the worker itself; its offline/CacheStorage journey passes under the opt-in. - master-search-header.tsx: close the desktop mode menu on Tab from the trigger (APG menu-button pattern). WebKit's sequential focus navigation can fail to deliver a wrapper-escaping focusout (links excluded from its Tab order; backward wrap into the menu), leaving the menu open. - ui-accessibility.spec.ts: webkit skip for the forced-colors token test — WebKit has no forced-colors implementation, so the media remap under test cannot engage there. - ui-formulation.spec.ts: pin the step-nav Previous button enabled with opacity 1 before the axe scan — WebKit can serve a stale :disabled computed style whose 0.4 opacity axe folds into a phantom contrast violation for a WCAG-1.4.3-exempt state. - ui-smoke.spec.ts: step over Firefox's scrollable-container tab stop (the sheet body sits between Close and 'New chat' in DOM order). - scripts/eval-retrieval.ts: ciwa -> ciwa-ar content alias. The matcher is whitespace-delimited, so the bare fixture term can never match the scale's hyphenated written name; canary runs #50/#51 ranked the CIWA-Ar dosing-table region top-5 yet the gate reported a miss. - tests/helpers/zero-touch.ts: shared stubZeroTouchPoints helper replacing the six inline copies from #995 (review follow-up). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
Summary
b9057f0: 716 passed / 28 failed — 25 WebKit, 3 Firefox). All 8ui-universal-searchWebKit failures share one signature: the typeahead suggestion content never enters the DOM ("element(s) not found" on the first dropdown assertion in every test).commandDropdownCanDisplay(src/lib/search-command-surface.ts, added by42a3e3con 2026-07-17 — after the last matrix run that ever completed, so it never executed on WebKit until run 4012) deliberately blocks wide touch devices: it requires the fine-pointer media query(hover: hover) and (pointer: fine)ornavigator.maxTouchPoints === 0. Headless CI browsers fail the fine-pointer query — the zero-touch escape hatch exists precisely so headless Chromium/Firefox (which report 0 touch points) still get the desktop dropdown. Playwright's Linux WebKit build, however, advertises phantom touch points on the touch-free runner, flunking both branches —useUniversalSearchis disabled (enabled: dropdownOpen && dropdownDisplayable && …) and no suggestion fetch ever fires.test.beforeEachinit script intests/ui-universal-search.spec.ts,tests/ui-smoke.spec.ts,tests/ui-tools.spec.ts,tests/ui-stress.spec.ts,tests/ui-accessibility.spec.ts, andtests/ui-formulation.spec.tsstubsNavigator.prototype.maxTouchPointsto the runner's true value (0). Applying it to all six ui specs makes the next matrix run cleanly separate gate-related failures from genuine WebKit bugs everywhere. The product gate and its unit pins (tests/search-command-surface.test.ts, including the explicitpointerMatches:false + maxTouchPoints:5 → blockedcase) are untouched — real iPhones/iPads stay blocked, real desktop Safari stays enabled. The stub is inert on Chromium/Firefox, which already report 0.:1244, the search request that never fires), several ui-smoke answer-composer flows, andui-stress:330(the documents-filter scope surface); explicitly not claimed areui-stress:409, the ui-accessibility focus/forced-colors cases, the ui-formulation contrast case,ui-tools:2087, and the three Firefox failures — of whichui-smoke:2932(@critical document search) fails on both Firefox and WebKit and is the highest-priority remainder (diagnosis in progress).Verification
npx prettier --checkon the six specs — cleannpx eslinton the six specs — cleannpm run typecheck— clean (afternpm installto sync the dependabot chore(deps-dev): add @typescript/typescript6 for AST tooling; hold typescript at 6.x (TS7 has no JS compiler API) #949@typescript/typescript6alias; a local-environment gap, not a repo issue)tests/ui-universal-search.spec.tsfails 16/20 in this container identically on unmodified main and with this change (stash/run/pop baseline) — a documented local-environment artifact; hosted CI Chromium is authoritative and was green for these tests in matrix run 4012UI verification not run beyond the above: WebKit cannot be installed in this container (repo policy); the WebKit delta is measured by the next
release-browser-matrixrun (dispatched immediately after this merges).Risk and rollout
Clinical Governance Preflight
Not applicable — Playwright test-environment correction only; no ingestion, answer generation, search/ranking logic, source rendering, document access, privacy, production env, or clinical output surface is touched (the ranking gate function itself is unmodified).
Notes
🤖 Generated with Claude Code
https://claude.ai/code/session_01EXsJcLrbZUXwnBeG91cVo9
Summary by CodeRabbit
Bug Fixes
Tests